Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update OKHTTP3 to v4.9.0 (v3.x -> v4.x) #14662

Closed
wants to merge 1 commit into from

Conversation

oztimpower
Copy link
Contributor

OkHTTP is referenced by the Quarkus BOM and is on a legacy v3 Java version. OkHTTP migrated fully to Kotlin in v4 with full backward compatibility to Java and v3.

https://square.github.io/okhttp/upgrading_to_okhttp_4/

@ghost ghost added the area/dependencies Pull requests that update a dependency file label Jan 28, 2021
@geoand
Copy link
Contributor

geoand commented Jan 28, 2021

I don't think we can, or want to make this bump.

The Kubernetes Client which AFAIR the main reason we have OkHTTP in the BOM is still on 3.x and doesn't plan to move to 4.x anytime soon.

@oztimpower
Copy link
Contributor Author

Agree that makes sense, however given that v4 makes an explicit effort to be fully backward compatible with v3, is it not worth trying? The functionality used by the K8s client (and others) would be standard functionality that should work, i.e. let the integration tests do their thing?

@geoand
Copy link
Contributor

geoand commented Jan 28, 2021

TBH, we are really against dragging in any sort of Kotlin related thing as a dependency.
I'm -1 but let's see what others say.

cc @iocanel @manusa @gsmet @maxandersen

@maxandersen
Copy link
Member

Yeah the kotlin stdlib dependency is problematic unfortunately :/

At the moment the alternatives are ranked by simplest to hardest (from my current POV :)

  • stay on okhttp v3
  • migrate away from okhttp v3 to vertx or other non problematic http client
  • find a way to convince everyone the kotlin stdlib is ok

@iocanel
Copy link
Contributor

iocanel commented Jan 28, 2021

One of the main reasons we picked okhttp as an http client in the first place, was that it didn't bring in lots of transitive dependencies. Unfortunately, this is no longer the case.

I have terrible experiences with frameworks bringing in general-purpose libraries (remember guava?) and I'd like to avoid that.
If I had the option I would even avoid using Jackson.

So, I am leaning towards using vertx.

@geoand
Copy link
Contributor

geoand commented Jan 28, 2021

A big +1 for Vert.x.

@manusa
Copy link
Contributor

manusa commented Jan 29, 2021

Hi, I have no idea about the implications of adding the Kotlin runtime to the build.

The claim from the project maintainers is that OkHttp4 is backwards compatible with OkHttp3. I'm not sure if any of the projects using Quarkus might have tried tweaking the dependency configuration in their pom.xml to make use of OkHttp4 and what the result was.

I really like the Vert.x idea (so +1). There would be a lot of work involved, + real breaking changes (especially for those who implemented custom interceptors, etc. for OkHttp), but at least we'd be depending on something we could rely on.

@gastaldi
Copy link
Contributor

I created a new issue in fabric8io/kubernetes-client#2764 requesting migration to the Vert.x HTTP Client.

@gastaldi
Copy link
Contributor

gastaldi commented Feb 1, 2021

Closing as upgrading it may not be a good idea given the reasons stated above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file triage/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants